Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Absorption #726

Merged
merged 6 commits into from
Feb 2, 2022
Merged

Absorption #726

merged 6 commits into from
Feb 2, 2022

Conversation

yang-ruoxi
Copy link
Contributor

@yang-ruoxi yang-ruoxi commented Dec 8, 2021 •
edited
Loading

Summary

  • Added AbsorptionFW to run a frequency-dependent dielectric calculation, with two options: independent-particle approximation (IPA) and random-phase approximation (RPA). One can also specify a static run, but it will invoke the MPStaticSet rather than the AbsorptionSet.
  • Added absorption task to write input sets for these runs
  • Added a section in vasp drones to collect the dielectrics information from RPA calculation, and to calculate the corresponding absorption coefficient for easy access.
  • A typical usage of this FW would be :
fw0 = OptimizeFW(structure)
fw1 = AbsorptionFW(mode = "STATIC", parent = fw0) # uses MPStaticSet
fw2 = AbsorptionFW(mode = "IPA", parent = fw1)   # uses AbsorptionSet supplied in pymatgen 
fw3 = AbsorptionFW(mode = "RPA", parent = fw2)  # uses AbsorptionSet supplied in pymatgen 
wf = Workflow([fw0, fw1, fw2, fw3])

@yang-ruoxi yang-ruoxi marked this pull request as ready for review December 10, 2021 00:05
@itsduowang
Copy link
Contributor

Hello @yang-ruoxi I think now you can run the workflows to test this PR. Please let me know for any problems or concerns.

Thanks for your contribution. Cheers! :)

@janosh
Copy link
Member

janosh commented Feb 1, 2022

Would be cool to get this merged!

@itsshawnzhang If @yang-ruoxi enabled edits by maintainers, you should be able to push a small change to this PR yourself and trigger CI that way.

@itsduowang
Copy link
Contributor

itsduowang commented Feb 1, 2022 •
edited
Loading

Would be cool to get this merged!

@itsshawnzhang If @yang-ruoxi enabled edits by maintainers, you should be able to push a small change to this PR yourself and trigger CI that way.

Thanks @janosh and @yang-ruoxi. I will push some changes to this PR and run the new CI at this point.

@janosh
Copy link
Member

janosh commented Feb 1, 2022

@itsshawnzhang Is allow edits from maintainers disabled? If so, it might be easier to add a the workflow_dispatch trigger to our test.yml GHA. I think that should allow you to trigger CI through a button and would avoid the need to create a duplicate PR.

workflow-dispatch-inputs

@itsduowang
Copy link
Contributor

@itsshawnzhang Is allow edits from maintainers disabled? If so, it might be easier to add a the workflow_dispatch trigger to our test.yml GHA. I think that should allow you to trigger CI through a button and would avoid the need to create a duplicate PR.

workflow-dispatch-inputs

Will try this! Thanks!

@janosh
Copy link
Member

janosh commented Feb 1, 2022

@itsshawnzhang This might be helpful.

- uses: actions/checkout@v2
  with:
    ref: pull/${{ github.event.pull_request.id }}/head

If you want to manually dispatch a Workflow for a Pull Request you could use workflow_dispatch with an input of the Pull Request id that the Workflow then composes the ref from (in the format above).

@itsduowang
Copy link
Contributor

This is helpful. Please allow me to look into it and I will create the check workflow for this PR. Thanks so much for the information @janosh :)

@itsduowang itsduowang mentioned this pull request Feb 1, 2022
Closed
@yang-ruoxi
Copy link
Contributor Author

Thanks @janosh and @yang-ruoxi. I will push some changes to this PR and run the new CI at this point.

Thanks for looking into it! Is there anything I should do from my end to merge it?

@janosh
Copy link
Member

janosh commented Feb 2, 2022

@yang-ruoxi If you could push any change to this PR (fix a typo or improve a doc string or something like that) to retrigger CI that would allow @itsshawnzhang to make sure it passes all tests before merging.

@janosh
Copy link
Member

janosh commented Feb 2, 2022

@yang-ruoxi The failing tests are due to pymongo v3/v4 incompatibilities. Can you merge or rebase the main branch into your PR?

@itsduowang itsduowang merged commit 2c91190 into hackingmaterials:main Feb 2, 2022
@itsduowang
Copy link
Contributor

Hello @yang-ruoxi . I merged this branch since all the tests are passed in the new environmental settings (see #744). Thank you so much for your contributions. Please let us know for any other problems or concerns. Cheers! : )

# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yang-ruoxi @itsduowang @janosh